refactor(tests): consolidate test tooling under tests/tools - #499
Conversation
All framework/runner code moves out of the suite trees into one flat package: - Standalone scripts: replay.py (smoke), prepare.py (editor E2E), stress.py (manual), plus compile_commands.py holding every compile_commands.json writer (cmake generation, static test data, per-test entries). These stay stdlib-only so the editor pixi env can run prepare.py without pygls. - Test library (was integration/utils): client.py (CliceClient), lifecycle.py (spawn/shutdown/clean-exit gate/ports), checks.py (diagnostics + anomaly + wait helpers), workspace.py (on-disk sources, document edits, cache inspection), injection.py. - conftest shrinks to fixtures and hooks; the cache_dir/worker-count test policy lives once in CliceClient.initialize — agentic tests that initialize directly now get the same 3-process default as fixture-based ones. - Resolves the tests/stress.py vs tests/integration/stress/ clash.
📝 WalkthroughWalkthroughThe PR consolidates integration-test helpers under ChangesTest utility consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/tools/compile_commands.py (1)
149-171: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a
stdparameter towrite_entriesfor parity withwrite_cdb.
write_cdballows overriding the C++ standard, butwrite_entrieshardcodes"-std=c++17". Minor inconsistency between the two sibling helpers; low priority since current callers only need c++17.♻️ Optional parity fix
-def write_entries(workspace, entries): +def write_entries(workspace, entries, *, std: str = "c++17"): """Write a compile_commands.json with per-file extra arguments. Args: workspace: Root directory of the workspace. entries: List of (file_name, extra_args) pairs; a file may appear multiple times to model multi-configuration projects. + std: C++ standard version (default: c++17). """ data = [ { "directory": str(workspace), "file": str(workspace / f), "arguments": [ "clang++", - "-std=c++17", + f"-std={std}", "-fsyntax-only", *args, str(workspace / f), ], } for f, args in entries ] (workspace / "compile_commands.json").write_text(json.dumps(data))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/tools/compile_commands.py` around lines 149 - 171, Add an optional std parameter to write_entries, defaulting to "c++17", and build the compiler argument from it instead of hardcoding "-std=c++17". Keep the existing behavior for current callers and align the parameter naming and handling with the sibling write_cdb helper.tests/tools/lifecycle.py (1)
121-145: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog swallowed exceptions during best-effort shutdown.
Static analysis flags the three
try/except Exception: passblocks inshutdown_client. The best-effort teardown intent is reasonable (a crashed/already-dead server shouldn't blow up teardown), but silently discarding every exception makes flaky CI teardown failures hard to diagnose.🔍 Suggested logging on swallowed exceptions
async def shutdown_client(c: CliceClient, *, verbose: bool = False) -> None: """Gracefully shut down a client, force-kill if needed.""" try: await asyncio.wait_for(c.shutdown_async(None), timeout=10.0) - except Exception: - pass + except Exception as exc: + print(f"[shutdown_client] shutdown_async failed: {exc!r}", flush=True) try: c.exit(None) - except Exception: - pass + except Exception as exc: + print(f"[shutdown_client] exit() failed: {exc!r}", flush=True) ... try: await assert_server_exited_cleanly(c.server) finally: try: await c.stop_io() await asyncio.sleep(0.1) - except Exception: - pass + except Exception as exc: + print(f"[shutdown_client] stop_io failed: {exc!r}", flush=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/tools/lifecycle.py` around lines 121 - 145, Update shutdown_client to log exceptions from each best-effort shutdown operation instead of silently passing: shutdown_async, exit, and stop_io. Preserve the existing teardown flow and exception swallowing, but emit concise diagnostic messages including the caught exception and identify which operation failed.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/tools/compile_commands.py`:
- Around line 149-171: Add an optional std parameter to write_entries,
defaulting to "c++17", and build the compiler argument from it instead of
hardcoding "-std=c++17". Keep the existing behavior for current callers and
align the parameter naming and handling with the sibling write_cdb helper.
In `@tests/tools/lifecycle.py`:
- Around line 121-145: Update shutdown_client to log exceptions from each
best-effort shutdown operation instead of silently passing: shutdown_async,
exit, and stop_io. Preserve the existing teardown flow and exception swallowing,
but emit concise diagnostic messages including the caught exception and identify
which operation failed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e44b4aef-bdf6-4a00-b7c7-5ee0c69020f9
📒 Files selected for processing (47)
.claude/CLAUDE.md.claude/commands/test.mddocs/en/dev/test-and-debug.mddocs/zh/dev/test-and-debug.mdeditors/vscode/.vscode-test.mjspixi.tomltests/conftest.pytests/integration/agentic/test_agentic.pytests/integration/agentic/test_cli.pytests/integration/compilation/test_header_pch.pytests/integration/compilation/test_pch.pytests/integration/compilation/test_persistent_cache.pytests/integration/compilation/test_self_containment.pytests/integration/compilation/test_staleness.pytests/integration/extensions/test_context_switching.pytests/integration/extensions/test_header_context.pytests/integration/features/test_completion.pytests/integration/features/test_file_tracker.pytests/integration/features/test_formatting.pytests/integration/features/test_guidance_diagnostics.pytests/integration/features/test_header_reindex.pytests/integration/features/test_inactive_regions.pytests/integration/features/test_index.pytests/integration/features/test_index_staleness.pytests/integration/features/test_query_freshness.pytests/integration/features/test_server.pytests/integration/lifecycle/test_anomaly.pytests/integration/lifecycle/test_config.pytests/integration/lifecycle/test_file_operation.pytests/integration/lifecycle/test_protocol_edges.pytests/integration/lifecycle/test_protocol_robustness.pytests/integration/modules/test_modules.pytests/integration/stress/test_eviction.pytests/integration/stress/test_rapid_edit.pytests/integration/utils/__init__.pytests/integration/utils/wait.pytests/integration/utils/workspace.pytests/tools/__init__.pytests/tools/checks.pytests/tools/client.pytests/tools/compile_commands.pytests/tools/injection.pytests/tools/lifecycle.pytests/tools/prepare.pytests/tools/replay.pytests/tools/stress.pytests/tools/workspace.py
💤 Files with no reviewable changes (3)
- tests/integration/utils/workspace.py
- tests/integration/utils/init.py
- tests/integration/utils/wait.py
Summary
All Python test framework/runner code moves out of the suite trees into one flat
tests/tools/package (net −32 lines despite two new modules), with duplication cleaned up along the way.Layout
tests/tools/top level stays stdlib-only importable: the editor pixi env (no pygls) runsprepare.py, which importscompile_commands—tests/tools/__init__.pyis deliberately empty.conftest.pyshrinks to fixtures and hooks (312 → 165 lines); lifecycle helpers move totests/tools/lifecycle.pywhere tests import them directly.CliceClient.initialize— agentic tests that initialize directly get the same 3-process default as fixture-based ones, and their redundant per-testinit_optionsare gone.stress.pyexplicitly opts back into server-side worker autoscaling (it exists to stress real pools).tests/stress.pyvstests/integration/stress/name clash; decorative section-separator comments removed.Test plan
editor-preparetask — all green locallytests/replay.pypaths in .claude docs; stress.py silently inheriting the 1-worker test default)Summary by CodeRabbit
Documentation
Tests